From 913c243fd467e29145656f9fcdd83611511bfb96 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Fri, 17 Oct 2014 11:48:57 -0700 Subject: [PATCH] Make SourceId cheap to clone Like PackageId, these are cloned quite often, so this moves them into an Arc to make them cheap to clone. This also removes the public fields in favor of accessors. --- src/cargo/core/package_id_spec.rs | 4 +- src/cargo/core/source.rs | 235 +++++++++++++---------- src/cargo/ops/cargo_rustc/fingerprint.rs | 2 +- src/cargo/sources/git/source.rs | 10 +- 4 files changed, 140 insertions(+), 111 deletions(-) diff --git a/src/cargo/core/package_id_spec.rs b/src/cargo/core/package_id_spec.rs index bfce8cd1e..7ce03616d 100644 --- a/src/cargo/core/package_id_spec.rs +++ b/src/cargo/core/package_id_spec.rs @@ -49,7 +49,7 @@ impl PackageIdSpec { PackageIdSpec { name: package_id.get_name().to_string(), version: Some(package_id.get_version().clone()), - url: Some(package_id.get_source_id().url.clone()), + url: Some(package_id.get_source_id().get_url().clone()), } } @@ -110,7 +110,7 @@ impl PackageIdSpec { } match self.url { - Some(ref u) => *u == package_id.get_source_id().url, + Some(ref u) => u == package_id.get_source_id().get_url(), None => true } } diff --git a/src/cargo/core/source.rs b/src/cargo/core/source.rs index 6b029b700..1e6df19d5 100644 --- a/src/cargo/core/source.rs +++ b/src/cargo/core/source.rs @@ -5,6 +5,7 @@ use std::fmt::{mod, Show, Formatter}; use std::hash; use std::iter; use std::mem; +use std::sync::Arc; use serialize::{Decodable, Decoder, Encodable, Encoder}; use url::Url; @@ -59,104 +60,26 @@ type Error = Box; /// Unique identifier for a source of packages. #[deriving(Clone, Eq)] pub struct SourceId { - pub url: Url, - pub kind: SourceKind, - // e.g. the exact git revision of the specified branch for a Git Source - pub precise: Option -} - -impl PartialOrd for SourceId { - fn partial_cmp(&self, other: &SourceId) -> Option { - self.to_string().partial_cmp(&other.to_string()) - } -} - -impl Ord for SourceId { - fn cmp(&self, other: &SourceId) -> Ordering { - self.to_string().cmp(&other.to_string()) - } -} - -impl> Encodable for SourceId { - fn encode(&self, s: &mut S) -> Result<(), E> { - if self.is_path() { - s.emit_option_none() - } else { - self.to_url().encode(s) - } - } -} - -impl> Decodable for SourceId { - fn decode(d: &mut D) -> Result { - let string: String = Decodable::decode(d).ok().expect("Invalid encoded SourceId"); - Ok(SourceId::from_url(string)) - } -} - -impl Show for SourceId { - fn fmt(&self, f: &mut Formatter) -> fmt::Result { - match *self { - SourceId { kind: PathKind, ref url, .. } => url.fmt(f), - SourceId { kind: GitKind(ref reference), ref url, ref precise, .. } => { - try!(write!(f, "{}", url)); - if reference.as_slice() != "master" { - try!(write!(f, "?ref={}", reference)); - } - - match *precise { - Some(ref s) => { - try!(write!(f, "#{}", s.as_slice().slice_to(8))); - } - None => {} - } - Ok(()) - }, - SourceId { kind: RegistryKind, ref url, .. } => { - let default = RegistrySource::url().ok(); - if default.as_ref() == Some(url) { - write!(f, "the package registry") - } else { - write!(f, "registry {}", url) - } - } - } - } + inner: Arc, } -// This custom implementation handles situations such as when two git sources -// point at *almost* the same URL, but not quite, even when they actually point -// to the same repository. -impl PartialEq for SourceId { - fn eq(&self, other: &SourceId) -> bool { - if self.kind != other.kind { return false } - if self.url == other.url { return true } - - match (&self.kind, &other.kind, &self.url, &other.url) { - (&GitKind(ref ref1), &GitKind(ref ref2), u1, u2) => { - ref1 == ref2 && - git::canonicalize_url(u1) == git::canonicalize_url(u2) - } - _ => false, - } - } -} - -impl hash::Hash for SourceId { - fn hash(&self, into: &mut S) { - self.kind.hash(into); - match *self { - SourceId { kind: GitKind(..), ref url, .. } => { - git::canonicalize_url(url).hash(into) - } - _ => self.url.hash(into), - } - } +#[deriving(Eq, Clone)] +struct SourceIdInner { + url: Url, + kind: SourceKind, + // e.g. the exact git revision of the specified branch for a Git Source + precise: Option } impl SourceId { pub fn new(kind: SourceKind, url: Url) -> SourceId { - SourceId { kind: kind, url: url, precise: None } + SourceId { + inner: Arc::new(SourceIdInner { + kind: kind, + url: url, + precise: None, + }), + } } /// Parses a source URL and returns the corresponding ID. @@ -198,12 +121,12 @@ impl SourceId { } pub fn to_url(&self) -> String { - match *self { - SourceId { kind: PathKind, .. } => { + match *self.inner { + SourceIdInner { kind: PathKind, .. } => { fail!("Path sources are not included in the lockfile, \ so this is unimplemented") }, - SourceId { + SourceIdInner { kind: GitKind(ref reference), ref url, ref precise, .. } => { let ref_str = if reference.as_slice() != "master" { @@ -220,7 +143,7 @@ impl SourceId { format!("git+{}{}{}", url, ref_str, precise_str) }, - SourceId { kind: RegistryKind, .. } => { + SourceIdInner { kind: RegistryKind, .. } => { // TODO: Central registry vs. alternates "registry+https://crates.io/".to_string() } @@ -255,15 +178,19 @@ impl SourceId { } pub fn get_url(&self) -> &Url { - &self.url + &self.inner.url + } + + pub fn get_kind(&self) -> &SourceKind { + &self.inner.kind } pub fn is_path(&self) -> bool { - self.kind == PathKind + self.inner.kind == PathKind } pub fn is_git(&self) -> bool { - match self.kind { + match self.inner.kind { GitKind(_) => true, _ => false } @@ -272,10 +199,10 @@ impl SourceId { /// Creates an implementation of `Source` corresponding to this ID. pub fn load<'a>(&self, config: &'a mut Config) -> Box { log!(5, "loading SourceId; {}", self); - match self.kind { + match self.inner.kind { GitKind(..) => box GitSource::new(self, config) as Box, PathKind => { - let path = match self.url.to_file_path() { + let path = match self.inner.url.to_file_path() { Ok(p) => p, Err(()) => fail!("path sources cannot be remote"), }; @@ -285,10 +212,112 @@ impl SourceId { } } + pub fn get_precise(&self) -> Option<&str> { + self.inner.precise.as_ref().map(|s| s.as_slice()) + } + pub fn with_precise(&self, v: String) -> SourceId { SourceId { - precise: Some(v), - .. self.clone() + inner: Arc::new(SourceIdInner { + precise: Some(v), + .. (*self.inner).clone() + }), + } + } +} + +impl PartialEq for SourceId { + fn eq(&self, other: &SourceId) -> bool { + self.inner.eq(&*other.inner) + } +} + +impl PartialOrd for SourceId { + fn partial_cmp(&self, other: &SourceId) -> Option { + self.to_string().partial_cmp(&other.to_string()) + } +} + +impl Ord for SourceId { + fn cmp(&self, other: &SourceId) -> Ordering { + self.to_string().cmp(&other.to_string()) + } +} + +impl> Encodable for SourceId { + fn encode(&self, s: &mut S) -> Result<(), E> { + if self.is_path() { + s.emit_option_none() + } else { + self.to_url().encode(s) + } + } +} + +impl> Decodable for SourceId { + fn decode(d: &mut D) -> Result { + let string: String = Decodable::decode(d).ok().expect("Invalid encoded SourceId"); + Ok(SourceId::from_url(string)) + } +} + +impl Show for SourceId { + fn fmt(&self, f: &mut Formatter) -> fmt::Result { + match *self.inner { + SourceIdInner { kind: PathKind, ref url, .. } => url.fmt(f), + SourceIdInner { kind: GitKind(ref reference), ref url, + ref precise, .. } => { + try!(write!(f, "{}", url)); + if reference.as_slice() != "master" { + try!(write!(f, "?ref={}", reference)); + } + + match *precise { + Some(ref s) => { + try!(write!(f, "#{}", s.as_slice().slice_to(8))); + } + None => {} + } + Ok(()) + }, + SourceIdInner { kind: RegistryKind, ref url, .. } => { + let default = RegistrySource::url().ok(); + if default.as_ref() == Some(url) { + write!(f, "the package registry") + } else { + write!(f, "registry {}", url) + } + } + } + } +} + +// This custom implementation handles situations such as when two git sources +// point at *almost* the same URL, but not quite, even when they actually point +// to the same repository. +impl PartialEq for SourceIdInner { + fn eq(&self, other: &SourceIdInner) -> bool { + if self.kind != other.kind { return false } + if self.url == other.url { return true } + + match (&self.kind, &other.kind, &self.url, &other.url) { + (&GitKind(ref ref1), &GitKind(ref ref2), u1, u2) => { + ref1 == ref2 && + git::canonicalize_url(u1) == git::canonicalize_url(u2) + } + _ => false, + } + } +} + +impl hash::Hash for SourceId { + fn hash(&self, into: &mut S) { + self.inner.kind.hash(into); + match *self.inner { + SourceIdInner { kind: GitKind(..), ref url, .. } => { + git::canonicalize_url(url).hash(into) + } + _ => self.inner.url.hash(into), } } } diff --git a/src/cargo/ops/cargo_rustc/fingerprint.rs b/src/cargo/ops/cargo_rustc/fingerprint.rs index 3f6f4e6a3..4027d3a2f 100644 --- a/src/cargo/ops/cargo_rustc/fingerprint.rs +++ b/src/cargo/ops/cargo_rustc/fingerprint.rs @@ -54,7 +54,7 @@ pub fn prepare_target(cx: &mut Context, pkg: &Package, target: &Target, // constant (which is the responsibility of the source) let use_pkg = { let doc = target.get_profile().is_doc(); - let path = match pkg.get_summary().get_source_id().kind { + let path = match *pkg.get_summary().get_source_id().get_kind() { PathKind => true, _ => false, }; diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs index b68a6e9ef..8938bac55 100644 --- a/src/cargo/sources/git/source.rs +++ b/src/cargo/sources/git/source.rs @@ -28,7 +28,7 @@ impl<'a, 'b> GitSource<'a, 'b> { config: &'a mut Config<'b>) -> GitSource<'a, 'b> { assert!(source_id.is_git(), "id is not git, id={}", source_id); - let reference = match source_id.kind { + let reference = match *source_id.get_kind() { GitKind(ref reference) => reference, _ => fail!("Not a git source; id={}", source_id) }; @@ -42,9 +42,9 @@ impl<'a, 'b> GitSource<'a, 'b> { let checkout_path = config.git_checkout_path() .join(ident.as_slice()).join(reference.as_slice()); - let reference = match source_id.precise { - Some(ref s) => s, - None => reference, + let reference = match source_id.get_precise() { + Some(s) => s, + None => reference.as_slice(), }; GitSource { @@ -160,7 +160,7 @@ impl<'a, 'b> Source for GitSource<'a, 'b> { let actual_rev = self.remote.rev_for(&self.db_path, self.reference.as_slice()); let should_update = actual_rev.is_err() || - self.source_id.precise.is_none(); + self.source_id.get_precise().is_none(); let (repo, actual_rev) = if should_update { try!(self.config.shell().status("Updating", -- 2.30.2